Skip to content

Add vpa crs for keda-operator and metrics-server#562

Open
QuantumEnigmaa wants to merge 4 commits intokedacore:mainfrom
QuantumEnigmaa:add-vpa-crs
Open

Add vpa crs for keda-operator and metrics-server#562
QuantumEnigmaa wants to merge 4 commits intokedacore:mainfrom
QuantumEnigmaa:add-vpa-crs

Conversation

@QuantumEnigmaa
Copy link
Copy Markdown

This PR adds VPA CRs for both keda-operator and metrics-server.

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • README is updated with new configuration values (if applicable) learn more
  • A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

Fixes #

Signed-off-by: QuantumEnigmaa <thibaud@giantswarm.io>
@QuantumEnigmaa QuantumEnigmaa requested a review from a team as a code owner November 9, 2023 08:57
Comment thread README.md Outdated
Comment thread README.md
Comment thread docs/index.yaml
Comment thread keda/Chart.yaml
Comment thread keda/values.yaml Outdated
@QuantumEnigmaa
Copy link
Copy Markdown
Author

I just noticed I forgot to sign my 2 previous commits :/

Copy link
Copy Markdown
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the new artifacts please?

Comment thread keda/values.yaml Outdated
Comment thread keda/README.md Outdated
Signed-off-by: QuantumEnigmaa <thibaud@giantswarm.io>
@QuantumEnigmaa
Copy link
Copy Markdown
Author

@tomkerkhove I reverted the changes on the artefacts

Comment thread keda/README.md

| Parameter | Type | Default | Description |
|-----------|------|---------|-------------|
| `autoscaling.verticalPodAutoscaler.keda.cpu.maxAllowed` | int | `2` | |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean I can get rid of this change in the PR as it will be automatically be run by the ci jobs ?

@JorTurFer
Copy link
Copy Markdown
Member

I think that this is fascinating, but I'm not sure if it works correctly before the live pod resizing is in GA. I mean, restarting the operator for updating the pod size could produce scaling downtimes and we should clarify it IMHO. This also could generate impact on k8s api server on huge clusters if the VPA scales the pod so often because all the scalers cache has to be rebuilt on each restart, recovering all the needed info from the cluster

@QuentinBisson
Copy link
Copy Markdown
Contributor

@JorTurFer considering live pod resizing is closing on in GA, do you think it would be a good time to revive this PR?

@JorTurFer
Copy link
Copy Markdown
Member

it's a good idea IMO, but probably we should open another PR from scratch based on this and including webhooks.
WDYT @kedacore/keda-helm-contributors ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants